Codecov Report

Merging #941 (5f425f6) into master (f8aaaeb) will not change coverage.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##           master     #941   +/-   ##
=======================================
  Coverage   98.72%   98.72%           
=======================================
  Files          30       30           
  Lines        1337     1337           
=======================================
  Hits         1320     1320           
  Misses         11       11           
  Partials        6        6           

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Thank, could you publish a release?

@zenthangplus Sure, I can. But can you wait until Monday, please? I'll need to verify the new version with Uber's Go Monorepo and we don't do releases on Friday to avoid causing issues over the weekend for services.

@sywhang i've tested, fx.Decorate work well, but seems fx.Replace is not work as expected. As i known, Replace is equivalent with Decorate.

@sywhang i've tested, fx.Decorate work well, but seems fx.Replace is not work as expected. As i known, Replace is equivalent with Decorate.

Can you be more specific? What's the code you wrote to test it?

@sywhang I've add two test cases, please take a look: #942

Oops, seems have some problem on master branch. When upgrading to latest code on master branch, my app is not run correctly, i don't know why, but i'm investigating.

I'm using the bellow function to handle request (for testing only).

httpHandler := func(w http.ResponseWriter, r *http.Request) {
		ctx := context.Background()
		ctxWithResponseWriter := context.WithValue(ctx, "w", w)
		ctxWithRequest := context.WithValue(ctxWithResponseWriter, "r", r)
		_ = app.Start(ctxWithRequest)
	}

and append hook

lc.Append(
		fx.Hook{
			OnStart: func(ctx context.Context) error {
				log.Info("start application") // Problem: only show once time
				r := ctx.Value("r").(*http.Request)
				w := ctx.Value("w").(*httptest.ResponseRecorder)
				httpServer.Handler.ServeHTTP(w, r)
				return nil
			},
		},
	)

But when run a test suite with two test cases, the last test case seems not run correctly. OnStart hook seems only run once time.

Then i've tried to revert to v1.18.1 and it works correctly. Seems latest code on master has problem.

Thanks for catching that @zenthangplus.
Would you mind creating a new issue for this? We nearly missed the comment.
If your investigation yields a reproduction of the bug, that would be really helpful for us to fix this.

@abhinav i've checked, it related to this MR #931, it not been released yet

Hey @zenthangplus, could you please specify what you mean? OnStart/OnStop hooks are only supposed to run once. If you were relying on it being run twice, that's not the intended behavior.

@sywhang In my case, i've wrote test for integration api testing. every http call to my api will be handled by this function.

httpHandler := func(w http.ResponseWriter, r *http.Request) {
		ctx := context.Background()
		ctxWithResponseWriter := context.WithValue(ctx, "w", w)
		ctxWithRequest := context.WithValue(ctxWithResponseWriter, "r", r)
		_ = app.Start(ctxWithRequest)
	}

Before it works well, but after this MR #931 app.Start is only allow to run once. I can change my logic to handle app.Stop before handle another request but i think this is breaking change and it break my test logic.

(again, it not related to this MR)

@zenthangplus I've created #945. Let's continue the discussion there.